Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

Pull request overview

This PR addresses JDBC v2 behavior when the SQL parser classifies a statement as UNKNOWN, ensuring Statement.execute(...) / PreparedStatement.execute() can still expose a ResultSet when the server returns one (e.g., for syntactically odd but valid SELECTs).

Changes:

• Treat UNKNOWN (and parse-error cases for ANTLR) as potentially result-set producing in SqlParserFacade.
• Adjust StatementImpl/PreparedStatementImpl execution behavior to return false when no ResultSet is produced and to throw from executeQuery(...) when no ResultSet exists.
• Add integration tests covering an “unknown” SELECT that still returns a ResultSet across SQL parser implementations.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

    public boolean execute() throws SQLException {
        ensureOpen();
        if (parsedPreparedStatement.isHasResultSet()) {
            currentResultSet = super.executeQueryImpl(buildSQL(), localSettings);
            return currentResultSet != null;
        } else {
            currentUpdateCount = super.executeUpdateImpl(buildSQL(), localSettings);
            return false;
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

This PR updates JDBC v2 statement execution behavior to better handle SQL that the client-side parser classifies as UNKNOWN/errored but ClickHouse can still execute successfully, ensuring execute()/executeQuery() comply with JDBC ResultSet/update-count semantics.

Changes:

• Treat UNKNOWN (JavaCC) and parse-error (ANTLR) statements as “result-set capable” so execute() can still expose a ResultSet when the server returns one.
• Adjust Statement.execute(...) / PreparedStatement.execute() to return false when no schema/result set is returned, and make executeQuery(...) throw when no ResultSet is produced.
• Add integration tests covering the “unknown SELECT still returns a ResultSet” scenario across parser implementations.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

This PR adjusts JDBC v2 execution semantics for statements that the SQL parser classifies as UNKNOWN/parse-error, so Statement.execute(...) / PreparedStatement.execute() can still expose a ResultSet when the server returns one, and correctly report “no result set” otherwise (per JDBC expectations).

Changes:

• Treat UNKNOWN (and ANTLR parse-error cases) as potentially result-set-producing during SQL parsing.
• Make execute() return false when the execution path yields no schema/ResultSet, and make executeQuery(...) throw when no ResultSet is produced.
• Add integration tests covering UNKNOWN/parse-error SELECT and no-ResultSet cases for both Statement and PreparedStatement.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

This PR adjusts JDBC v2 execution semantics for SQL statements that the client-side parser classifies as UNKNOWN (or encounters parse errors), so execute(...)/execute() can still surface a ResultSet when the server returns one, while keeping JDBC behavior correct when no ResultSet is produced.

Changes:

• Treat UNKNOWN statements (and ANTLR parse-error cases) as potentially returning a ResultSet in SqlParserFacade.
• Tighten Statement.executeQuery(...) / PreparedStatement.executeQuery() to throw when no ResultSet is produced, and make execute(...) return false when execution yields no schema.
• Add integration tests covering UNKNOWN-select and no-ResultSet paths (DDL/INSERT).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

    public boolean execute() throws SQLException {
        ensureOpen();
        currentUpdateCount = -1;
        if (parsedPreparedStatement.isHasResultSet()) {
            currentResultSet = super.executeQueryImpl(buildSQL(), localSettings);
            return currentResultSet != null;
        } else {
            currentUpdateCount = super.executeUpdateImpl(buildSQL(), localSettings);
            return false;
        }

    public boolean execute(String sql) throws SQLException {
        ensureOpen();
        parsedStatement = connection.getSqlParser().parsedStatement(sql);
        currentUpdateCount = -1;
        if (parsedStatement.isHasResultSet()) {
            currentResultSet = executeQueryImpl(sql, localSettings);
            return currentResultSet != null;
        } else {
            currentUpdateCount = executeUpdateImpl(sql, localSettings);
            postUpdateActions();
            return false;
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.